Skip to content

Add styling json file - #14058

Merged
kaamyashinde merged 2 commits into
mainfrom
add-styling-json-file
Jul 31, 2026
Merged

Add styling json file#14058
kaamyashinde merged 2 commits into
mainfrom
add-styling-json-file

Conversation

@kaamyashinde

@kaamyashinde kaamyashinde commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

These JSON files will be used in the styling system that will be setup

Issue
Resolves #14048

Approach
Added the two json files that were sent by the EDS team. Parent folder was also renamed to styling and sub folders were introduced based on the role of the files

Screenshot 2026-07-31 at 10 56 18
  • PR title captures the intent of the changes, and is fitting for release notes.
  • Added appropriate release note label
  • Commit history is consistent and clean, in line with the contribution guidelines.
  • Make sure unit tests pass locally after every commit (git rebase -i main --exec 'just rapid-tests')

When applicable

  • When screenshots are changed: Review screenshot-PR in ert-testdata,
    merge screenshot-PR in ert-testdata before merging this PR.
  • When there are user facing changes: Updated documentation
  • New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
  • Large PR: Prepare changes in small commits for more convenient review
  • Bug fix: Add regression test for the bug
  • Bug fix: Add backport label to latest release (format: 'backport release-branch-name')

@kaamyashinde kaamyashinde linked an issue Jul 30, 2026 that may be closed by this pull request
@kaamyashinde
kaamyashinde force-pushed the add-styling-json-file branch from 7ac6480 to 4a14e4d Compare July 30, 2026 13:59
@kaamyashinde kaamyashinde self-assigned this Jul 30, 2026
@SAKavli

SAKavli commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Looking good!
I am not convinced by my own opinion, but what do you think of the structure:
/styles/themes/{dark,light}.json ?
Are dark/light considered styles or themes?
And in the future we will add style objects, how do these fit into the folder structure?
Would also love @eilskra 's opinion!

@codecov-commenter

codecov-commenter commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.81%. Comparing base (d9fe003) to head (0fc52e1).
⚠️ Report is 6 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14058      +/-   ##
==========================================
- Coverage   91.84%   91.81%   -0.04%     
==========================================
  Files         480      480              
  Lines       33337    33402      +65     
==========================================
+ Hits        30619    30667      +48     
- Misses       2718     2735      +17     
Flag Coverage Δ
cli-tests 36.24% <0.00%> (-0.11%) ⬇️
fuzz 44.08% <100.00%> (-0.04%) ⬇️
gui-tests 58.33% <0.00%> (-0.06%) ⬇️
performance-and-unit-tests 80.42% <100.00%> (+0.03%) ⬆️
test 46.41% <0.00%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/ert/gui/styling/__init__.py 100.00% <100.00%> (ø)
src/ert/gui/styling/theme_manager/manager.py 100.00% <ø> (ø)
src/ert/gui/styling/theme_manager/theme.py 100.00% <ø> (ø)

... and 10 files with indirect coverage changes

@kaamyashinde

kaamyashinde commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author
Screenshot 2026-07-30 at 16 11 03

we currently have the manager.py which is the theme manager that is responsible for translating the json for qt to be able to processed. So perhaps the whole folder should be restructured in some way.

perhaps something like the following, where the future .qss.in file that applies the styling can be added inside the qss_styling folder, in case we want to have one file per component. So f.eks

Screenshot 2026-07-30 at 16 16 23

Additionally, we can drop the qss_styling folder as of now and just place the .qss.in file inside the styling folder.

@SAKavli @eilskra

@eilskra

eilskra commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

I vote either themes or maybe color_schemes?
e.g /styles/themes/{dark,light}.json

Components could maybe just go under maybe style_sheets or something, maybe something along the lines of:

styles/
-- color_schemes/
---- {dark,light}.json
-- style_sheets/
---- qss.in 

@eilskra

eilskra commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

perhaps something like the following, where the future .qss.in file that applies the styling can be added inside the qss_styling folder, in case we want to have one file per component. So f.eks

Or like that, I vote this 👍

@codspeed-hq

codspeed-hq Bot commented Jul 30, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 36 untouched benchmarks


Comparing add-styling-json-file (0fc52e1) with main (793ddcd)

Open in CodSpeed

Renamed from theming to styling and grouped together the logic files in theme_manager to prepare for adding themes and color tokens
@kaamyashinde
kaamyashinde force-pushed the add-styling-json-file branch from 4a14e4d to 0fc52e1 Compare July 31, 2026 08:22
@kaamyashinde
kaamyashinde marked this pull request as ready for review July 31, 2026 08:46
@kaamyashinde
kaamyashinde requested review from SAKavli and eilskra July 31, 2026 08:57

@eilskra eilskra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! 🥇

@kaamyashinde
kaamyashinde merged commit 576dcf6 into main Jul 31, 2026
38 checks passed
@kaamyashinde
kaamyashinde deleted the add-styling-json-file branch July 31, 2026 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add styling json file

4 participants